Automatically generate table of contents in text #2213
Conversation
|
just a heads up, I probably won't get around to deeply reviewing this until next week. at a glance it looks good though. |
There was a problem hiding this comment.
It passed my stress test, leaving behind just a nitpick.
Your code is high quality, especially buildToc which addresses the complexity of the problem in an elegant way.
There's only the problem of {:toc} rendering in comments, but even then, we're talking about UX that can be fixed easily.
I also talked about the duplication issue, but that doesn't depend from your code, nor was in scope for this PR/issue, just something to keep in mind.
It works really well, nice job! ^^
|
|
||
| return toc | ||
| }, [text]) | ||
| const toc = useMemo(() => extractHeadings(text), [text]) |
There was a problem hiding this comment.
I noticed that we don't give IDs to nodes inside comments, only on full posts:
stacker.news/components/text.js
Line 116 in 4620160
Because of this, the table of contents won't work if used in comments. I personally think that the ToC doesn't make that much sense in comments, so we can just disable {:toc} for them with topLevel awareness. What do you think? ^^
There was a problem hiding this comment.
I agree, it wouldn't make sense to use Toc in comments, so I changed it so that remarkToc is only processed for topLevel items
| const str = toString(node) | ||
| headings.push({ | ||
| heading: str, | ||
| slug: slug(str.replace(/[^\w\-\s]+/gi, '')), |
There was a problem hiding this comment.
note for the future, unrelated to review:
it seems that we don't handle duplicate headings, because we use slug instead of the GithubSlugger class.
Probably because it's faster as GithubSlugger would have instead tracked headings in memory and checked every heading against the previous ones to count 🤔.
There was a problem hiding this comment.
We don't use GithubSlugger because of #1405
There was a problem hiding this comment.
Ok, will leave this as is for now
Description
Closes #2208
Writing
{:toc}on its own line will automatically render a table of contents as a bulleted list, using the headings (#,##, etc) contained in the post.Screenshots
auto-toc.mov
Additional Context
n/a
Checklist
Are your changes backwards compatible? Please answer below:
yes
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
{:toc}somewhere it could render in unexpected ways.The behavior of links that start with a hash (i.e.
#section-1) has also changed, but I can't see why anyone would've wanted such links to open in a new tab.For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a
Did you introduce any new environment variables? If so, call them out explicitly here:
no